Change large card view to use full width#4110
Conversation
|
Not a fan of this, especially for wide devices. Then you get stuff like this on really wide devices: 
If you want to increase the double column breakpoint from |
|
You mean that images get cut off? Doesnt happen in my testing, it always shows the full image. Although theres a problem that tall images can take up the entire screen, possibly needs a max height. 
Using |
Yes this is the case, and its why wider screens should be using the 2-column layout.
xl should be >1200 px wide: https://getbootstrap.com/docs/5.3/layout/breakpoints/#available-breakpoints There's several places where this should be done I think, but I'll test it out. |
|
I've verified that |
|
Ah making the browser window narrower does switch it to single column. But 
With the change in this PR you can see larger images also on big screens, and dont need to expand every single image: |
That's an issue with the top level containers not something to be fixed with this PR. The card view should not be ignoring the double columns, that's something that I want to keep. |
|
The problem is that "Card" and "Small Card" modes are almost identical on large screens. While "Card" does show images a bit larger, its not large enough to view them properly, and so you still need to expand manually for every single image. |
| url.searchParams.set("max_size", bannerSize.toString()); | ||
| break; | ||
| case "full_size": | ||
| break; |
There was a problem hiding this comment.
Noticed a bug here, in large card mode it was always loading images downscaled to 512px because it was entering the default case here. Also added other variants where it seems to make sense.
There was a problem hiding this comment.
Hrm... not sure about this one. 512px seems plenty large enough, and we def want to downscale no matter, even for the card views, to save on initial page load bandwidth. These images could be 10MB or more, we don't know.
The only time we shouldn't be downscaling IMO, is for the full image viewer.
There was a problem hiding this comment.
Okay changed the name to large_thumbnail and set it to 2048px.
| case "card": | ||
| return "col-12"; | ||
| case "small_card": | ||
| return "col-12 col-md-6"; |
There was a problem hiding this comment.
Just change this:
| return "col-12 col-md-6"; | |
| return "col-12 col-xl-6"; |
The cards should still be doing a 2-column masonry layout, its what differentiates it from a list view.
| url.searchParams.set("max_size", bannerSize.toString()); | ||
| break; | ||
| case "full_size": | ||
| break; |
There was a problem hiding this comment.
Hrm... not sure about this one. 512px seems plenty large enough, and we def want to downscale no matter, even for the card views, to save on initial page load bandwidth. These images could be 10MB or more, we don't know.
The only time we shouldn't be downscaling IMO, is for the full image viewer.
I'm not really sure that's the case: 
This to me is plenty large enough for scrolling, and my screen isn't even that large. |
|
Sure the text is still barely readable. But if you use Card view on mobile it shows images practically in fullscreen size. Then switch to desktop with the same mode and suddenly you only get medium-sized thumbnails. In the Lemmy settings you can only specify a single Listing Mode which applies to all devices. So it should actually have similar functionality everywhere, otherwise you need to change Listing Mode all the time. |
|
If ppl want an ultra-wide view, that's what In the future we could possibly make the number of columns configurable, but IMO that's a bad idea, because it should be done automatically by the current device screen size. We should also probably even add 3 columns or more for |
|
More columns makes no sense if the container has such a limited width. On large screen with browser in fullscreen, Lemmy only uses about half the available width. |
2 participants
This seems better to view full-size images on desktop, without separately expanding each image.